Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Add vcpop.m/vfirst.m to RISCVMaskedPseudosTable #115162

Conversation

wangpc-pp
Copy link
Contributor

We seem to forget these two instructions.

Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
wangpc-pp added a commit that referenced this pull request Nov 6, 2024
Created using spr 1.3.6-beta.1
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 6, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Pengcheng Wang (wangpc-pp)

Changes

We seem to forget these two instructions.


Full diff: https://github.com/llvm/llvm-project/pull/115162.diff

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (+12-2)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td (+1)
  • (modified) llvm/test/CodeGen/RISCV/rvv/fixed-vectors-reduction-fp.ll (+2-8)
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 6291842e071a3e..17b617c502ca90 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -3708,6 +3708,15 @@ static bool isImplicitDef(SDValue V) {
   return V.getMachineOpcode() == TargetOpcode::IMPLICIT_DEF;
 }
 
+static bool hasGPROut(unsigned Opc) {
+  switch (RISCV::getRVVMCOpcode(Opc)) {
+  case RISCV::VCPOP_M:
+  case RISCV::VFIRST_M:
+    return true;
+  }
+  return false;
+}
+
 // Optimize masked RVV pseudo instructions with a known all-ones mask to their
 // corresponding "unmasked" pseudo versions. The mask we're interested in will
 // take the form of a V0 physical register operand, with a glued
@@ -3737,8 +3746,9 @@ bool RISCVDAGToDAGISel::doPeepholeMaskedRVV(MachineSDNode *N) {
 #endif
 
   SmallVector<SDValue, 8> Ops;
-  // Skip the passthru operand at index 0 if !UseTUPseudo.
-  for (unsigned I = !UseTUPseudo, E = N->getNumOperands(); I != E; I++) {
+  // Skip the passthru operand at index 0 if !UseTUPseudo and no GPR out.
+  bool ShouldSkip = !UseTUPseudo && !hasGPROut(Opc);
+  for (unsigned I = ShouldSkip, E = N->getNumOperands(); I != E; I++) {
     // Skip the mask, and the Glue.
     SDValue Op = N->getOperand(I);
     if (I == MaskOpIdx || Op.getValueType() == MVT::Glue)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
index 89e71b7c22c12d..d5c7932b6f8edf 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -1150,6 +1150,7 @@ class VPseudoUnaryNoMaskGPROut :
 class VPseudoUnaryMaskGPROut :
       Pseudo<(outs GPR:$rd),
              (ins VR:$rs1, VMaskOp:$vm, AVL:$vl, sew:$sew), []>,
+      RISCVMaskedPseudo<MaskIdx=1>,
       RISCVVPseudo {
   let mayLoad = 0;
   let mayStore = 0;
diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-reduction-fp.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-reduction-fp.ll
index ca17ea49a6f920..487234674befe0 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-reduction-fp.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-reduction-fp.ll
@@ -1797,11 +1797,8 @@ define float @vreduce_fminimum_v7f32(ptr %x) {
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    vsetivli zero, 7, e32, m2, ta, ma
 ; CHECK-NEXT:    vle32.v v8, (a0)
-; CHECK-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
-; CHECK-NEXT:    vmset.m v0
-; CHECK-NEXT:    vsetivli zero, 7, e32, m2, ta, ma
 ; CHECK-NEXT:    vmfne.vv v10, v8, v8
-; CHECK-NEXT:    vcpop.m a0, v10, v0.t
+; CHECK-NEXT:    vcpop.m a0, v10
 ; CHECK-NEXT:    beqz a0, .LBB111_2
 ; CHECK-NEXT:  # %bb.1:
 ; CHECK-NEXT:    lui a0, 523264
@@ -2558,11 +2555,8 @@ define float @vreduce_fmaximum_v7f32(ptr %x) {
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    vsetivli zero, 7, e32, m2, ta, ma
 ; CHECK-NEXT:    vle32.v v8, (a0)
-; CHECK-NEXT:    vsetivli zero, 8, e8, mf2, ta, ma
-; CHECK-NEXT:    vmset.m v0
-; CHECK-NEXT:    vsetivli zero, 7, e32, m2, ta, ma
 ; CHECK-NEXT:    vmfne.vv v10, v8, v8
-; CHECK-NEXT:    vcpop.m a0, v10, v0.t
+; CHECK-NEXT:    vcpop.m a0, v10
 ; CHECK-NEXT:    beqz a0, .LBB139_2
 ; CHECK-NEXT:  # %bb.1:
 ; CHECK-NEXT:    lui a0, 523264

Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I double checked and we're setting ElementsDependOnVL and ElementsDependOnMask for VCPOP_M and VFIRST_M so adding RISCVMaskedPseudo should be safe.

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td Outdated Show resolved Hide resolved
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
@wangpc-pp wangpc-pp changed the base branch from users/wangpc-pp/spr/main.riscv-add-vcpopmvfirstm-to-riscvmaskedpseudostable to main November 7, 2024 07:41
@wangpc-pp wangpc-pp merged commit 3850801 into main Nov 7, 2024
10 of 11 checks passed
@wangpc-pp wangpc-pp deleted the users/wangpc-pp/spr/riscv-add-vcpopmvfirstm-to-riscvmaskedpseudostable branch November 7, 2024 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants